[clang][DebugInfo][NFC] Add createConstantValueExpression helper #70674
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch factors out the code to create a DIExpression from an APValue into a separate helper function. This will be useful in a follow-up patch where we re-use this logic elsewhere.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Michael Buch (Michael137) ChangesThis patch factors out the code to create a DIExpression from an APValue into a separate helper function. This will be useful in a follow-up patch where we re-use this logic elsewhere. Pre-requisite for #70639 Full diff: https://github.com/llvm/llvm-project/pull/70674.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
index 0aaf678bf287c6e..9de55e41f885d26 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5580,25 +5580,8 @@ void CGDebugInfo::EmitGlobalVariable(const ValueDecl *VD, const APValue &Init) {
auto &GV = DeclCache[VD];
if (GV)
return;
- llvm::DIExpression *InitExpr = nullptr;
- if (CGM.getContext().getTypeSize(VD->getType()) <= 64) {
- // FIXME: Add a representation for integer constants wider than 64 bits.
- if (Init.isInt()) {
- const llvm::APSInt &InitInt = Init.getInt();
- std::optional<uint64_t> InitIntOpt;
- if (InitInt.isUnsigned())
- InitIntOpt = InitInt.tryZExtValue();
- else if (auto tmp = InitInt.trySExtValue(); tmp.has_value())
- // Transform a signed optional to unsigned optional. When cpp 23 comes,
- // use std::optional::transform
- InitIntOpt = (uint64_t)tmp.value();
- if (InitIntOpt)
- InitExpr = DBuilder.createConstantValueExpression(InitIntOpt.value());
- } else if (Init.isFloat())
- InitExpr = DBuilder.createConstantValueExpression(
- Init.getFloat().bitcastToAPInt().getZExtValue());
- }
+ llvm::DIExpression *InitExpr = createConstantValueExpression(VD, Init);
llvm::MDTuple *TemplateParameters = nullptr;
if (isa<VarTemplateSpecializationDecl>(VD))
@@ -5935,3 +5918,28 @@ llvm::DINode::DIFlags CGDebugInfo::getCallSiteRelatedAttrs() const {
return llvm::DINode::FlagAllCallsDescribed;
}
+
+llvm::DIExpression *
+CGDebugInfo::createConstantValueExpression(clang::ValueDecl const *VD,
+ const APValue &Val) {
+ llvm::DIExpression *ValExpr = nullptr;
+ if (CGM.getContext().getTypeSize(VD->getType()) <= 64) {
+ // FIXME: Add a representation for integer constants wider than 64 bits.
+ if (Val.isInt()) {
+ const llvm::APSInt &ValInt = Val.getInt();
+ std::optional<uint64_t> ValIntOpt;
+ if (ValInt.isUnsigned())
+ ValIntOpt = ValInt.tryZExtValue();
+ else if (auto tmp = ValInt.trySExtValue(); tmp.has_value())
+ // Transform a signed optional to unsigned optional. When cpp 23 comes,
+ // use std::optional::transform
+ ValIntOpt = (uint64_t)tmp.value();
+ if (ValIntOpt)
+ ValExpr = DBuilder.createConstantValueExpression(ValIntOpt.value());
+ } else if (Val.isFloat())
+ ValExpr = DBuilder.createConstantValueExpression(
+ Val.getFloat().bitcastToAPInt().getZExtValue());
+ }
+
+ return ValExpr;
+}
diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index ae12485850ca775..7b60e94555d0608 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -800,6 +800,11 @@ class CGDebugInfo {
llvm::MDTuple *&TemplateParameters,
llvm::DIScope *&VDContext);
+ /// Create a DIExpression representing the constant corresponding
+ /// to the specified 'Val'. Returns nullptr on failure.
+ llvm::DIExpression *createConstantValueExpression(const clang::ValueDecl *VD,
+ const APValue &Val);
+
/// Allocate a copy of \p A using the DebugInfoNames allocator
/// and return a reference to it. If multiple arguments are given the strings
/// are concatenated.
|
felipepiovezan
approved these changes
Oct 30, 2023
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
| llvm::DIExpression * | ||
| CGDebugInfo::createConstantValueExpression(clang::ValueDecl const *VD, | ||
| const APValue &Val) { | ||
| llvm::DIExpression *ValExpr = nullptr; |
There was a problem hiding this comment.
I know this is how the original code was written, but since we are making a separate function we are now in a position where we can delete this line, early-return on the if and replace all assignments to ValExpr with return statements.
dwblaikie
reviewed
Oct 30, 2023
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
Comment on lines
5939
to
5945
| if (ValIntOpt) | ||
| return DBuilder.createConstantValueExpression(ValIntOpt.value()); | ||
| } else if (Val.isFloat()) | ||
| return DBuilder.createConstantValueExpression( | ||
| Val.getFloat().bitcastToAPInt().getZExtValue()); | ||
|
|
||
| return nullptr; |
There was a problem hiding this comment.
could modify this a bit to simplify indentation, maye:
if (Val.isFloat())
return DBUilder.createConstantValueExpression(...);
const llvm::APSInt &ValInt = Val.getInt();
...
if (ValIntOpt)
return DBuilder.createConstantValueExpression(*ValIntOpt);
return nullptr;
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
| std::optional<uint64_t> ValIntOpt; | ||
| if (ValInt.isUnsigned()) | ||
| ValIntOpt = ValInt.tryZExtValue(); | ||
| else if (auto tmp = ValInt.trySExtValue(); tmp.has_value()) |
There was a problem hiding this comment.
This is a bit redundant/equivalent to:
else if (auto tmp = ValInt.trySExtValue())
clang/lib/CodeGen/CGDebugInfo.cpp
Outdated
| else if (auto tmp = ValInt.trySExtValue(); tmp.has_value()) | ||
| // Transform a signed optional to unsigned optional. When cpp 23 comes, | ||
| // use std::optional::transform | ||
| ValIntOpt = (uint64_t)tmp.value(); |
There was a problem hiding this comment.
I'd probably write this as (uint64_t)*tmp
dwblaikie
approved these changes
Oct 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch factors out the code to create a DIExpression from an APValue into a separate helper function.
This will be useful in a follow-up patch where we re-use this logic elsewhere.
Pre-requisite for #70639